Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating bar mask offsets directly from pySIAF #509

Merged
merged 2 commits into from
Dec 9, 2021

Conversation

kammerje
Copy link

@kammerje kammerje commented Dec 8, 2021

Based on discussions with Marshall Perrin and Mario Gennaro we noticed that WebbPSF uses outdated bar mask offsets for the NIRCam bar mask coronagraphs. We have now updated WebbPSF to grab the most recent offsets directly from pySIAF.

Copy link
Collaborator

@JarronL JarronL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty straightforward. A few suggestions, including a couple comments comparing new values as well as a way of improving performance.

webbpsf/optics.py Outdated Show resolved Hide resolved
webbpsf/optics.py Outdated Show resolved Hide resolved
webbpsf/optics.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mperrin mperrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good, thanks @kammerje! Some relatively minor suggestions below.

webbpsf/optics.py Outdated Show resolved Hide resolved
webbpsf/optics.py Show resolved Hide resolved
webbpsf/optics.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 8, 2021

Codecov Report

Merging #509 (f2a123a) into develop (1789c8c) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #509      +/-   ##
===========================================
+ Coverage    65.33%   65.41%   +0.07%     
===========================================
  Files           12       12              
  Lines         4922     4933      +11     
===========================================
+ Hits          3216     3227      +11     
  Misses        1706     1706              
Impacted Files Coverage Δ
webbpsf/optics.py 59.75% <100.00%> (+0.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1789c8c...f2a123a. Read the comment docs.

@mperrin
Copy link
Collaborator

mperrin commented Dec 8, 2021

@JarronL, not directly related to this PR, but at some point maybe you could help enlighten us on how the wide filter positions are calculated? I don't understand how the wavelengths are weighted for the broadband. It seems to me the W filters are all systematically shifted towards the wider edge of the mask compared to the M filters, and that's not obvious to me why that should be the case. In particular, I'd expect broadband filter averages to be tilted toward shorter wavelengths, because there's more stellar photons at those wavelengths.

In any case we should use the values in the SIAF; it's just for my own understanding I'd like to better follow the logic for how these are derived.

@JarronL
Copy link
Collaborator

JarronL commented Dec 8, 2021

Hmm... this is more a John Stansberry questions, but I'll give it a shot...

According to the NIRCam Coronagraphic Ops document (NIRCam_CoronagraphOps_V4.pdf), the wide band filters are treated a little differently compared to the medium band filters. Medium band filter locations take into account targets with stellar spectral types. But, the wide band "equations must be modified to account for the longer wavelengths they admit relative to those admitted by the F210M and F460M filters. For this purpose, the reference wavelength of interest is defined by the long half-transmission point, lambda_1/2, of the F210M and F460M filters, which are 2.196 and 4.747 um respectively."

For instance, the long-half-power point for F444W is 4.992, so the location was calculated as -3.16 * (4.992 - 4.747) = -0.7742. This is from 2016, though, so things have obviously changed somewhat given the slight differences relative to the pysiaf values.

In short, I guess they wanted to be sure that the longer wavelength contribution from the filters were fully occulted even if that meant the mask position is oversized for the shorter wavelength portions of the filter.

@kammerje
Copy link
Author

kammerje commented Dec 9, 2021

Thanks, I added all of your suggested changes.

@kammerje kammerje closed this Dec 9, 2021
@kammerje kammerje reopened this Dec 9, 2021
@JAStansberry
Copy link

The use of a different (longer) effective wavelength for calculating the offsets for the W filters came from John Krist, based on modeling he did.
It would actually be useful to use the current version of WebbPSF to model the contrast as a function of displacement along the bar. John was using a pretty notional OPD map, I think.

Also, looking back at my script for calculating the offsets, the were based on pixel scales of 32 and 65 mas. My guess is that the difference due to that probably can't be measured, but we may as well use the right values. Even better, use the scale appropriate to the location of the bar masks and taking into account the new distortion model for the coronagraph FOV.

@mperrin
Copy link
Collaborator

mperrin commented Dec 9, 2021

Thank you @kammerje! Much appreciated. I'll go ahead and merge this now.

@JAStansberry and @JarronL, we could indeed do some contrast calculations for different MASKLWB offsets as you propose. (I'm about to finally update the OPD maps for as-built latest & greatest, too, so these would be much better than earlier notional OPDs). That's probably better done as a task by Jarron or someone else on the NIRCam IDT rather than any of us in the webbpsf or telescope teams - I'd be happy to advise on how to do the modeling though. It sounds like the choices John Krist made may have been motivated by getting as deep as possible contrast, at a cost of inner working angle. Which may well be the right call but it's something to clarify and convey to users.

Tagging @juliengirard too - FYI see the conversation on this PR about MASKLWB offsets.

@mperrin mperrin self-requested a review December 9, 2021 15:30
@mperrin mperrin merged commit e5cd611 into spacetelescope:develop Dec 9, 2021
@JarronL
Copy link
Collaborator

JarronL commented Dec 9, 2021

Sounds good. @mperrin, if you could make a new PR and tag me as the responsible, I can look into it ::waves hand vaguely:: sometime in the near future. Would like to understand how the most recent filter-dependent bar mask reference positions as defined in SIAF were calculated, though, which probably requires some combination of @mgennaro, @JAStansberry, @juliengirard.

@mperrin
Copy link
Collaborator

mperrin commented Dec 9, 2021

Done - see #511, @JarronL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants